📋 Review Summary

This PR implements intelligent tool parallelism based on tool Kind, replacing the hard-coded Agent-only concurrent execution model. The implementation is well-structured, with clear separation of concerns and comprehensive test coverage. The changes enable read-only tools (Read, Search, Fetch, Think) to execute in parallel while preserving ordering semantics for mutating tools (Edit, Execute).

🔍 General Feedback

• Clean architecture: The helper functions (isConcurrencySafe, partitionToolCalls, runConcurrently) are well-isolated and follow single-responsibility principles.
• Good documentation: JSDoc comments clearly explain the batching behavior with concrete examples.
• Thoughtful design: The consecutive batching approach preserves implicit ordering that models may rely on while maximizing parallelism opportunities.
• Configurable concurrency: The QWEN_CODE_MAX_TOOL_CONCURRENCY environment variable provides operational flexibility.
• Test coverage: The updated test properly validates that all concurrency-safe tools run in parallel rather than just Agent tools.

🎯 Specific Feedback

🟡 High

• File: packages/core/src/core/coreToolScheduler.ts:344-349 - The isConcurrencySafe function checks ToolNames.AGENT by name but other tools by kind. This creates an inconsistency in how concurrency safety is determined. Consider whether Agent should also have a Kind.Agent value added to CONCURRENCY_SAFE_KINDS for consistency, or document why Agent is treated as a special case.

• File: packages/core/src/core/coreToolScheduler.ts:358-369 - The partitionToolCalls logic groups consecutive safe tools but creates a sequential batch for each unsafe tool. This means [Read, Edit, Read] becomes [[Read](parallel), [Edit](seq), [Read](seq)] - the second Read could potentially run in parallel with the first if they're both safe. However, the current design intentionally preserves ordering, which is correct but worth documenting more explicitly that this is a deliberate ordering guarantee, not a limitation.

🟢 Medium

• File: packages/core/src/core/coreToolScheduler.ts:1353-1371 - The runConcurrently function uses a Set to track executing promises and removes them via .then() callback. However, if a promise rejects before completion, the .then() callback may not execute properly. Consider using .finally() instead to ensure cleanup:

  const p = this.executeSingleToolCall(call, signal).finally(() => {
    executing.delete(p);
  });

• File: packages/core/src/tools/tools.ts:740-746 - The CONCURRENCY_SAFE_KINDS set is exported but only used internally by coreToolScheduler.ts. Consider whether this should be exported or kept internal to reduce API surface. If exported, consider adding a comment explaining the safety guarantees (no side effects, no shared mutable state).

• File: packages/core/src/core/coreToolScheduler.ts:344 - The comment mentions "no side effects, no shared mutable state" but the isConcurrencySafe function doesn't verify these properties - it relies on the Kind classification. Consider adding a stronger comment that tool implementers must ensure their Kind-appropriate tools actually satisfy these guarantees.

🔵 Low

• File: packages/core/src/core/coreToolScheduler.test.ts:3147 - The test timeout was increased from 20ms to 50ms for the read tool. Consider using a more descriptive constant or configuration value to make the test intent clearer (e.g., const TOOL_EXECUTION_DELAY = 50).

• File: packages/core/src/core/coreToolScheduler.ts:334 - The decorative separator line (// ─── Tool Concurrency Helpers ────────────────────────────────) uses Unicode characters that may not render consistently across all editors. Consider using standard ASCII separators for better compatibility.

• File: packages/core/src/test-utils/mock-tool.ts:27 - The kind?: Kind option is added but there's no validation that the provided kind matches the tool's actual behavior. Consider adding a comment or validation in test utilities to ensure test authors don't misclassify mock tools.

✅ Highlights

• Excellent test update: The test at line 3122 properly validates that ALL concurrency-safe tools (not just Agent) run in parallel, with clear assertions checking that all starts happen before any ends.
• Smart batching strategy: The partitionToolCalls function elegantly handles the complex scenario of interleaved safe/unsafe tools while preserving ordering guarantees.
• Environment variable configuration: Adding QWEN_CODE_MAX_TOOL_CONCURRENCY shows operational maturity and allows tuning for different environments.
• Minimal changes: The implementation achieves significant functionality with focused changes to only 4 files, demonstrating good modular design.
• Type safety: The use of ReadonlySet<Kind> and proper TypeScript typing ensures the concurrency-safe kinds list is immutable and type-checked.